Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CP-4498: Added relaxed check mode to allow iSCSI SRs during XSM #1116

Merged
merged 1 commit into from Apr 23, 2013

Conversation

BobBall
Copy link
Contributor

@BobBall BobBall commented Mar 25, 2013

Signed-off-by: Bob Ball bob.ball@citrix.com

@xen-git
Copy link
Contributor

xen-git commented Mar 25, 2013

Can one of the admins verify this patch?

@djs55
Copy link
Contributor

djs55 commented Mar 26, 2013

Ok to test

On Monday, March 25, 2013, xen-git wrote:

Can one of the admins verify this patch?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1116#issuecomment-15408344
.

Dave Scott

try
let dest_vdi_ref = XenAPI.VDI.get_by_uuid remote_rpc session_id vdi_uuid in
let dest_vdi_sr_ref = XenAPI.VDI.get_SR remote_rpc session_id dest_vdi_ref in
if dest_vdi_sr_ref = dest_sr_ref then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to be testing to see whether the SR references are the same or the uuids? I would have thought uuid would be better in that I can see a way to ensuring this is the same between pools, whereas I can't see how the references would ever be the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's checking that the VDI requested exists on the SR that we've told XSM we want to migrate to.

dest_sr_ref is provided to the migrate_send function and must be the remote ref (personally I think this really should have been the UUID to remove uncertainties) - dest_vdi_sr_ref is therefore the SR which the VDI already exists on, and the check is that the target VDI is on the correct SR on the remote end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the sr uuid is also checked at line 292

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, thanks.

@ghost ghost assigned jonludlam Mar 26, 2013
@xen-git
Copy link
Contributor

xen-git commented Mar 27, 2013

Can one of the admins verify this patch?

@jonludlam
Copy link
Contributor

ok to test

@jonludlam
Copy link
Contributor

Can we put in a strong warning or even error if we believe that the SR uuids match but the VDI doesn't exist on the other side? That's a situation we ought to be really worried about :-)

@djs55
Copy link
Contributor

djs55 commented Apr 2, 2013

If the VDI doesn't exist we could do an SR.scan, since xapi's view might
just be out of date. If its still not there then something very bad is
happening (eg old mirror?)

On Tuesday, April 2, 2013, Jon Ludlam wrote:

Can we put in a strong warning or even error if we believe that the SR
uuids match but the VDI doesn't exist on the other side? That's a situation
we ought to be really worried about :-)


Reply to this email directly or view it on GitHubhttps://github.com//pull/1116#issuecomment-15768456
.

Dave Scott

@BobBall
Copy link
Contributor Author

BobBall commented Apr 2, 2013

Added an error and an SR scan. Because these are both exceptional cases I'm not sure how to test them - so I'm hoping a visual inspection will suffice.

Signed-off-by: Bob Ball bob.ball@citrix.com
@jonludlam
Copy link
Contributor

OK, looks good now!

@BobBall
Copy link
Contributor Author

BobBall commented Apr 23, 2013

So what needs to happen next for it to be merged?

@jonludlam
Copy link
Contributor

Me to click the button :-) we had a minor freeze last week for a Linux 3.x change to go in, but we've thawed now. I'll merge now.

jonludlam pushed a commit that referenced this pull request Apr 23, 2013
CP-4498: Added relaxed check mode to allow iSCSI SRs during XSM
@jonludlam jonludlam merged commit 92b1d56 into xapi-project:master Apr 23, 2013
@BobBall
Copy link
Contributor Author

BobBall commented Apr 23, 2013

Thanks Jon!

@BobBall BobBall deleted the cp-4498 branch April 23, 2013 11:14
n0ano pushed a commit to n0ano/gantt that referenced this pull request Dec 11, 2013
Fix for bug 1160323.

DocImpact
Depends on a version of XAPI supporting relax-xsm-sr-check.
Currently no release versions support this, so the change at
xapi-project/xen-api#1116 would need to be applied
to the source to enable this.  XenServer 6.2 is expected to
support this mode, and possibly some future hotfixes for XenServer 6.1.

It also depends on a configuration change which must be documented and added to devstack:
* Set "relax-xsm-sr-check = true" in /etc/xapi.conf

This commit makes the following changes:
* Attach the SR on the destination host prior to migrate
* Returns the SR-ref from the pre_migrate call (API change)
* Populates the XS VDI map with the sr-ref on the destination host
* Removes the connection to the SR from the source host post-migrate
* Added plugin to determine if iSCSI block migration is enabled

Associated tempest test at https://review.openstack.org/#/c/26478/

Change-Id: I917d9cf094190d636f4b9e14f6c8e728ff85af0e
Fixes: bug 1160323
n0ano pushed a commit to n0ano/ganttclient that referenced this pull request Dec 13, 2013
Fix for bug 1160323.

DocImpact
Depends on a version of XAPI supporting relax-xsm-sr-check.
Currently no release versions support this, so the change at
xapi-project/xen-api#1116 would need to be applied
to the source to enable this.  XenServer 6.2 is expected to
support this mode, and possibly some future hotfixes for XenServer 6.1.

It also depends on a configuration change which must be documented and added to devstack:
* Set "relax-xsm-sr-check = true" in /etc/xapi.conf

This commit makes the following changes:
* Attach the SR on the destination host prior to migrate
* Returns the SR-ref from the pre_migrate call (API change)
* Populates the XS VDI map with the sr-ref on the destination host
* Removes the connection to the SR from the source host post-migrate
* Added plugin to determine if iSCSI block migration is enabled

Associated tempest test at https://review.openstack.org/#/c/26478/

Change-Id: I917d9cf094190d636f4b9e14f6c8e728ff85af0e
Fixes: bug 1160323
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants